Add BuildInformation record with Version flexible parsing and open issues support#6
Conversation
…on parsing Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…se detection Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Malcolmnixon
left a comment
There was a problem hiding this comment.
It looks like the code wants to get the open issues from the RepoConnector, but there's no API for it. Please add an API then implement the BuildInformation connector to use it appropriately.
Additionally it seems like the approach to parsing version information from tag names should be improved. Consider creating a Tag Version record which takes a tag name and exposes fields to get:
- Original tag name
- Full Version (major.minor.patch-prerelease) stripping leading junk
- IsPreRelease (pre-release is specified)
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
I've addressed all the feedback:
Changes in commits 6faee5a, e12ecba. All 58 tests passing, CodeQL clean (0 alerts). |
Malcolmnixon
left a comment
There was a problem hiding this comment.
I think TagVersion should be renamed TagInformation, and I think the RepoConnector APIs should be updated to return instances of TagInformation where appropriate.
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Done. Renamed Changes in commits 2816495, 53e9180. All 58 tests passing, CodeQL clean (0 alerts). |
src/DemaConsulting.BuildMark/RepoConnectors/MockRepoConnector.cs
Outdated
Show resolved
Hide resolved
src/DemaConsulting.BuildMark/RepoConnectors/MockRepoConnector.cs
Outdated
Show resolved
Hide resolved
…n, update APIs to use TagInfo, add varied tag formats Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Malcolmnixon
left a comment
There was a problem hiding this comment.
The Tag parsing seems to believe that ".arch" and ".linux" should be considered metadata - that doesn't seem to align with the Semantic Version 2.0.0 format - instead metadata like that would probably come after a "+" separator.
…arators with build metadata support Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Updated to align with the flexible versioning format. The regex now:
Examples working correctly:
Changes in commit cfd89e2. All 15 TagInfo tests passing. |
| /// <param name="Tag">The tag name.</param> | ||
| /// <param name="FullVersion">The full semantic version (major.minor.patch-prerelease) with leading non-version characters removed.</param> | ||
| /// <param name="IsPreRelease">Whether this is a pre-release version.</param> | ||
| public partial record TagInfo(string Tag, string FullVersion, bool IsPreRelease) |
There was a problem hiding this comment.
Let's save the Version, Pre-Release and Metadata as part of the record; so we have [Tag, FullVersion, Version, PreRelease, Metadata, IsPreRelease] . These extra fields will probably be valuable later when Markdown reports are generated.
There was a problem hiding this comment.
Added all requested properties to the Version record:
Tag: Original tag nameFullVersion: Complete version stringSemanticVersion: Core version (major.minor.patch)PreRelease: Pre-release identifierMetadata: Build metadataIsPreRelease: Boolean flag
Changes in commit b2c6b32.
| /// <param name="BugIssues">Bugs fixed between versions.</param> | ||
| /// <param name="KnownIssues">Known issues (unfixed or fixed but not in this build).</param> | ||
| public record BuildInformation( | ||
| string? FromVersion, |
There was a problem hiding this comment.
The FromVersion and ToVersion should also probably be TagInfo... but again I wonder about the TagInfo name... it's really turning into a "Version" as the version can also come from the command-line rather than only coming from tags. Let's rename TagInfo to "Version".
There was a problem hiding this comment.
Renamed TagInfo to Version throughout the codebase. BuildInformation now uses Version? for FromVersion and Version for ToVersion.
Changes in commit b2c6b32.
| /// <param name="tag">Tag information (null for current state).</param> | ||
| /// <returns>Git hash.</returns> | ||
| Task<string> GetHashForTagAsync(string? tag); | ||
| Task<string> GetHashForTagAsync(TagInfo? tag); |
There was a problem hiding this comment.
Actually lets put this method back to taking a string, because it may be used to get hashes for any type of tag, not just a Version tag.
There was a problem hiding this comment.
Reverted GetHashForTagAsync to accept string? instead of Version?. This allows it to work with any tag type, not just version tags.
Changes in commit b2c6b32.
| /// <param name="toTag">Ending tag (null for current state).</param> | ||
| /// <returns>List of pull request IDs.</returns> | ||
| Task<List<string>> GetPullRequestsBetweenTagsAsync(string? fromTag, string? toTag); | ||
| Task<List<string>> GetPullRequestsBetweenTagsAsync(TagInfo? fromTag, TagInfo? toTag); |
There was a problem hiding this comment.
This method does want to take the "Version" records as its got a definite from/to version vibe, but lets rename the parameters to "from" and "to" to drop the tag.
There was a problem hiding this comment.
Updated GetPullRequestsBetweenTagsAsync parameter names from fromTag/toTag to from/to. The method still accepts Version? parameters as requested.
Changes in commit b2c6b32.
…operties, update API parameters Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot Quality checks failed, please run and fix |
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot The unit tests are failing, please run and fix. |
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Build Information Record - All Tests Passing
cspell "**/*.{md,cs}"- PASSES (0 issues)markdownlint "**/*.md"- PASSESyamllint '**/*.{yml,yaml}'- PASSESdotnet build --configuration Release- PASSES (0 warnings)dotnet test --configuration Release- PASSES (61/61)Fixed test expectations:
BuildInformation_CreateAsync_ReleaseSkipsPreReleasesto expect.Tagproperty value "v2.0.0" (not "2.0.0")BuildInformation_CreateAsync_SeparatesBugAndChangeIssuesto expect 2 change issues (feature + documentation)BuildInformation_CreateAsync_WorksWhenCurrentCommitMatchesLatestTagto expect "ver-1.1.0" as FromVersionMockRepoConnectorMatchingTagto use consistent tag namesAll tests now passing across net8.0, net9.0, and net10.0.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.